Skip to content

Conversation

@marcindsobczak
Copy link

Addressing #705


* This method MUST NOT be exposed on public-facing RPC APIs.

* It is strongly recommended that this method, and its `testing_` namespace, be disabled by default. Enabling it should require an explicit command-line flag or configuration setting by the node operator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this fit better under debug_ namespace?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of creating a new namespace is to prevent enabling this method by mistake, together with other debug ones.
It was specified here: ethpandaops/gas-lighting-tracker#5
This new endpoint is sensitive and should exist under a new, non-exposed namespace (i.e., not debug_ or admin_ which are sometimes exposed by RPC providers).

@spencer-tb has another idea - to make it _debug so it will be different than current debug, but will not introduce quite exotic testing namespace.
I'm open for every option.


* The client MUST include all transactions from the `transactions` array on the block's transaction list, in the order they were provided.

* The client MUST NOT include any transactions from its local transaction pool. The resulting block MUST only contain the transactions specified in the `transactions` array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The client MUST NOT include any transactions from its local transaction pool. The resulting block MUST only contain the transactions specified in the `transactions` array.
* If the `transactions` array is non-empty, the client MUST NOT include any transactions from its local transaction pool. The resulting block MUST only contain the transactions specified in the `transactions` array.

It might be a useful (optional?) feature to allow for selectiung txpool transactions in the case where the transaction array is empty.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it as simple as possible in V1 to make it as easy as possible to implement for all EL teams. The use case is simple here - to build a block from provided txs. Your proposition is interesting, but:

  1. It can break some test cases e.g. empty payload tests
  2. It will add complexity - in current form we can totally ignore mempool and it's transactions

Also here, if majority of people will opt to have such feature, I'm fine with adding it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants